Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: move from semaphore to rwmutex #215

Merged
merged 2 commits into from
Nov 21, 2023

Conversation

zach-source
Copy link
Contributor

Description of your changes

Replaced the semaphore in the internal terraform harness with a RWMutex. RWMutex protects the terraform shared cache from corruption. If an init is performed, it requires a write lock. Only one write lock at a time. If another action is performed, a read lock is acquired. More than one read locks can be acquired. This prevents an init from inadvertently changing a plugin while it's in use. Prevents issues with text file busy errors. This is required if using max-reconcile-rate greater than 1.

Fixes #186

I have:

  • Run make reviewable to ensure this PR is ready for review.

How has this code been tested

I've tested the code locally with a max reconcile rate of 4.

Signed-off-by: Zachary Taylor <zctaylor.work@gmail.com>
@Upbound-CLA
Copy link

Upbound-CLA commented Nov 20, 2023

CLA assistant check
All committers have signed the CLA.

Signed-off-by: Zachary Taylor <zctaylor.work@gmail.com>
@ytsarev
Copy link
Member

ytsarev commented Nov 20, 2023

/test-examples="examples/workspace-inline-aws.yaml"

Copy link
Collaborator

@bobh66 bobh66 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thanks!

@bobh66 bobh66 merged commit a358e66 into upbound:main Nov 21, 2023
10 checks passed
@zach-source zach-source deleted the fix/cacheAccessRaceCond branch November 24, 2023 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Terraform provider processes hanging
5 participants